feat(hugr-cli)!: move mermaid to own sub-command #1390

ss2165 · 2024-08-02T10:18:17Z

drive-by replace clap_stdin with clio as that's that clap recommends, and it comes with stdout too.

BREAKING CHANGE: Cli validate command no longer has a mermaid option, use mermaid sub-command instead.

doug-q

Doesn't it make sense to validate the Hugr AND mermaid-print it? I would not be surprised if mermaid-printing an invalid Hugr caused a panic. Will the lack of update_validate to replace OpaqueOp with ExtensionOp and do extension inference(I think? not sure of the state of both of these) cause problems?

ss2165 · 2024-08-02T10:38:04Z

Yes perhaps, the change is because validate --mermaid --no-validate is just a confusing command to me.
I can common up the validation so mermaid also calls it, and add the --no-validate back to the mermaid command. Does that suit?

aborgna-q · 2024-08-02T10:55:34Z

I would not be surprised if mermaid-printing an invalid Hugr caused a panic.

It shouldn't, as long as the underlying portgraph is working correctly.
I think not having rendering fail for unrelated errors is a good thing.

more used, recommended, maintained

less confusing

ss2165 · 2024-08-02T10:58:27Z

I can common up the validation so mermaid also calls it,

In latest commit if you want to look at it. Though broadly I agree with @aborgna-q and would prefer to revert it.

codecov · 2024-08-02T11:02:24Z

Codecov Report

Attention: Patch coverage is 77.27273% with 10 lines in your changes missing coverage. Please review.

Project coverage is 87.64%. Comparing base (f78c0cc) to head (2f7034e).

Files	Patch %	Lines
hugr-cli/src/mermaid.rs	50.00%	2 Missing and 3 partials ⚠️
hugr-cli/src/lib.rs	82.35%	2 Missing and 1 partial ⚠️
hugr-cli/src/validate.rs	86.66%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1390      +/-   ##
==========================================
- Coverage   87.64%   87.64%   -0.01%     
==========================================
  Files         117      118       +1     
  Lines       20329    20340      +11     
  Branches    18024    18035      +11     
==========================================
+ Hits        17818    17826       +8     
+ Misses       1725     1724       -1     
- Partials      786      790       +4

Flag	Coverage Δ
python	`91.67% <ø> (ø)`
rust	`87.12% <77.27%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

doug-q · 2024-08-02T11:19:00Z

I would not be surprised if mermaid-printing an invalid Hugr caused a panic.

It shouldn't, as long as the underlying portgraph is working correctly. I think not having rendering fail for unrelated errors is a good thing.

This is convincing. My preference is an option to pass --validate to mermaid. I am partly motivated by wanting to have this option in other cli tools.

doug-q

Very nice, thank you.

## 🤖 New release * `hugr`: 0.10.0 -> 0.11.0 * `hugr-core`: 0.7.0 -> 0.8.0 * `hugr-passes`: 0.6.2 -> 0.7.0 * `hugr-cli`: 0.3.0 -> 0.4.0 <details><summary>Changelog</summary> ## `hugr` <blockquote> ## 0.11.0 (2024-08-12) ### Bug Fixes - [**breaking**] BasicBlockExits should not be `OpTag::DataflowParent` ([#1409](#1409)) ### Documentation - Clarify CustomConst::equal_consts ([#1396](#1396)) ### Features - [**breaking**] Serialised extensions ([#1371](#1371)) - Serialised standard extensions ([#1377](#1377)) - [**breaking**] Update remaining builder methods to "infer by default" ([#1386](#1386)) - Add Eq op to logic extension ([#1398](#1398)) - Improve error message on failed custom op validation ([#1416](#1416)) - [**breaking**] `Extension` requires a version ([#1367](#1367)) </blockquote> ## `hugr-core` <blockquote> ## 0.8.0 (2024-08-12) ### Bug Fixes - [**breaking**] BasicBlockExits should not be `OpTag::DataflowParent` ([#1409](#1409)) ### Documentation - Clarify CustomConst::equal_consts ([#1396](#1396)) ### Features - [**breaking**] `Extension` requires a version ([#1367](#1367)) - [**breaking**] Serialised extensions ([#1371](#1371)) - Serialised standard extensions ([#1377](#1377)) - [**breaking**] Update remaining builder methods to "infer by default" ([#1386](#1386)) - Add Eq op to logic extension ([#1398](#1398)) - Improve error message on failed custom op validation ([#1416](#1416)) </blockquote> ## `hugr-passes` <blockquote> ## 0.7.0 (2024-08-12) ### Features - [**breaking**] `Extension` requires a version ([#1367](#1367)) </blockquote> ## `hugr-cli` <blockquote> ## 0.4.0 (2024-08-12) ### Features - Serialised standard extensions ([#1377](#1377)) - Validate with extra extensions and packages ([#1389](#1389)) - [**breaking**] Move mermaid to own sub-command ([#1390](#1390)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). --------- Co-authored-by: Craig Roy <[email protected]>

🤖 I have created a release *beep* *boop* --- ## [0.6.0](hugr-py-v0.5.0...hugr-py-v0.6.0) (2024-08-12) ### ⚠ BREAKING CHANGES * **hugr-py:** Moved `hugr.get_serialization_version` to `hugr.serialization.serial_hugr.serialization_version` * **hugr-cli:** Cli validate command no longer has a mermaid option, use `mermaid` sub-command instead. * `TypeDefBound` uses struct-variants for serialization. `SignatureFunc` now has variants for missing binary functions, and serializes in to a new format that indicates expected binaries. ### Features * `Package` pydantic model for modules + extensions ([#1387](#1387)) ([68cfac5](68cfac5)), closes [#1358](#1358) * Define `Const` inline by default, and add a parameter to change the parent ([#1404](#1404)) ([3609736](3609736)) * **hugr-cli:** move mermaid to own sub-command ([#1390](#1390)) ([77795b9](77795b9)) * **hugr-py:** add type_bound method to `Type` ([#1410](#1410)) ([bd5ba47](bd5ba47)), closes [#1365](#1365) * **hugr-py:** Allow defining functions, consts, and aliases inside DFGs ([#1394](#1394)) ([d554072](d554072)) * **hugr-py:** Reexport commonly used classes from the package root ([#1393](#1393)) ([69925d0](69925d0)) * **py:** `Hugr.to_json` and `.load_json` helpers ([#1403](#1403)) ([e7f9f4c](e7f9f4c)) * **py:** Allow pre-declaring a `Function`'s output types ([#1417](#1417)) ([fa0f5a4](fa0f5a4)) * **py:** implement `iter` on `ToNode` ([#1399](#1399)) ([e88910b](e88910b)) * **py:** Parametric int type helper, and arbitrary width int constants ([#1406](#1406)) ([abd70c9](abd70c9)) * Serialised extensions ([#1371](#1371)) ([31be204](31be204)) ### Bug Fixes * **py:** `Hugr.__iter__` returning `NodeData | None` instead of `Node`s ([#1401](#1401)) ([c134584](c134584)) * **py:** Set output cont for Conditionals ([#1415](#1415)) ([67bb8a0](67bb8a0)) ### Documentation * **hugr-py:** expand toctree ([#1411](#1411)) ([aa81c9a](aa81c9a)) * **hugr-py:** remove multiversion + add justfile command ([#1381](#1381)) ([dd1dc48](dd1dc48)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

ss2165 requested a review from a team as a code owner August 2, 2024 10:18

ss2165 requested review from doug-q and removed request for a team August 2, 2024 10:18

ss2165 force-pushed the ss/cli-merm branch from 801e265 to d1be045 Compare August 2, 2024 10:28

doug-q reviewed Aug 2, 2024

View reviewed changes

Base automatically changed from ss/package-cli to main August 2, 2024 10:54

ss2165 added 5 commits August 2, 2024 11:57

refactor(hugr-cli): switch to clio rather than clap-stdin

826c951

more used, recommended, maintained

rename validate::CliArgs to ValArgs

376fa97

less confusing

refactor(hugr-cli): common up some args

1a74ec3

feat(hugr-cli)!: move mermaid to own sub-command

06bd1f3

by default validate

ee35bbe

ss2165 force-pushed the ss/cli-merm branch from d1be045 to ee35bbe Compare August 2, 2024 10:57

ss2165 changed the title ~~feat(hugr-cli)!: move mermaid to own sub-command~~ feat!(hugr-cli): move mermaid to own sub-command Aug 2, 2024

ss2165 changed the title ~~feat!(hugr-cli): move mermaid to own sub-command~~ feat!(hugr-cli): move mermaid to own sub-command Aug 2, 2024

ss2165 changed the title ~~feat!(hugr-cli): move mermaid to own sub-command~~ feat(hugr-cli)!: move mermaid to own sub-command Aug 2, 2024

fixup python cli calls

2f7034e

make validation optional

deaadec

ss2165 requested a review from doug-q August 2, 2024 11:22

doug-q approved these changes Aug 2, 2024

View reviewed changes

ss2165 added this pull request to the merge queue Aug 2, 2024

Merged via the queue into main with commit 77795b9 Aug 2, 2024
20 checks passed

ss2165 deleted the ss/cli-merm branch August 2, 2024 12:54

This was referenced Aug 2, 2024

chore(main): release hugr-py 0.6.0 #1379

Merged

chore: release #1376

Merged

hugrbot mentioned this pull request Aug 13, 2024

chore: release #1420

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(hugr-cli)!: move mermaid to own sub-command #1390

feat(hugr-cli)!: move mermaid to own sub-command #1390

ss2165 commented Aug 2, 2024

doug-q left a comment

ss2165 commented Aug 2, 2024

aborgna-q commented Aug 2, 2024

ss2165 commented Aug 2, 2024

codecov bot commented Aug 2, 2024 •

edited

Loading

doug-q commented Aug 2, 2024

doug-q left a comment

feat(hugr-cli)!: move mermaid to own sub-command #1390

feat(hugr-cli)!: move mermaid to own sub-command #1390

Conversation

ss2165 commented Aug 2, 2024

doug-q left a comment

Choose a reason for hiding this comment

ss2165 commented Aug 2, 2024

aborgna-q commented Aug 2, 2024

ss2165 commented Aug 2, 2024

codecov bot commented Aug 2, 2024 • edited Loading

Codecov Report

doug-q commented Aug 2, 2024

doug-q left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 2, 2024 •

edited

Loading